Skip to content

fix: consider basePath when generating slide URLs#2384

Open
coaxial wants to merge 4 commits into
slidevjs:mainfrom
coaxial:basepath
Open

fix: consider basePath when generating slide URLs#2384
coaxial wants to merge 4 commits into
slidevjs:mainfrom
coaxial:basepath

Conversation

@coaxial
Copy link
Copy Markdown
Contributor

@coaxial coaxial commented Nov 25, 2025 •
edited
Loading

Hello,

This fixes an issue with the presenter when using build --base=/path/ where the presenter will instead generate slide links as /<slide number> instead of /path/<slide number>.

I've added a test as well but getting this error when trying to run the test suite:

$ pnpm test

> @52.9.1 test /home/pierre/code/slidev
> vitest test


 DEV  v4.0.8 /home/pierre/code/slidev

 ✓ test/transform-magic-move.test.ts (2 tests) 537ms
   ✓ basic  381ms
 ✓ test/parser.test.ts (74 tests) 114ms
 ✓ packages/parser/src/timesplit/timesplit.test.ts (1 test) 16ms
 ✓ test/transform.test.ts (8 tests) 18ms
 ✓ packages/parser/src/timesplit/timestring.test.ts (2 tests) 9ms
 ✓ test/transform-all.test.ts (1 test) 12ms

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Failed Suites 1 ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯

 FAIL  test/utils.test.ts [ test/utils.test.ts ]
Error: Cannot find module '#slidev/slides' imported from '/home/pierre/code/slidev/packages/client/logic/slides.ts'
 ❯ packages/client/logic/slides.ts:2:24
      1| import type { SlideRoute } from '@slidev/types'
      2| import { slides } from '#slidev/slides'
       |                        ^
      3| import { computed, watch, watchEffect } from 'vue'
      4| import { useNav } from '../composables/useNav'
 ❯ test/utils.test.ts:4:1

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯


 Test Files  1 failed | 6 passed (7)
      Tests  88 passed (88)
   Start at  21:40:47
   Duration  1.99s (transform 557ms, setup 0ms, collect 1.29s, tests 706ms, environment 2ms, prepare 66ms)

I haven't changed this import and without my patch (i.e. just after cloning), the tests run fine.

@netlify
Copy link
Copy Markdown

netlify Bot commented Nov 25, 2025 •
edited
Loading

✅ Deploy Preview for slidev ready!

Name Link
🔨 Latest commit 1e056a4
🔍 Latest deploy log https://app.netlify.com/projects/slidev/deploys/69e08f58a5552c0008f74c40
😎 Deploy Preview https://deploy-preview-2384--slidev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Nov 25, 2025 •
edited
Loading

Open in StackBlitz

@slidev/client

npm i https://pkg.pr.new/@slidev/client@2384

create-slidev

npm i https://pkg.pr.new/create-slidev@2384

create-slidev-theme

npm i https://pkg.pr.new/create-slidev-theme@2384

@slidev/parser

npm i https://pkg.pr.new/@slidev/parser@2384

@slidev/cli

npm i https://pkg.pr.new/@slidev/cli@2384

@slidev/types

npm i https://pkg.pr.new/@slidev/types@2384

commit: 1e056a4

@coaxial coaxial changed the title Consider basePath when generating slide URLs fix: consider basePath when generating slide URLs Nov 25, 2025
antfu
antfu reviewed Nov 27, 2025
View reviewed changes
Comment thread test/utils.test.ts
import type { ResolvedFontOptions, SlideInfo, SlideRoute } from '@slidev/types'
import { relative, resolve } from 'node:path'
import { slash } from '@antfu/utils'
import { getSlidePath } from '@slidev/client/logic/slides'
Copy link
Copy Markdown
Member

@antfu antfu Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is in client environment that relies on virtual modules; we can't directly test it in this file like this. We might want to extract this logic to test spareately

@kermanx
Copy link
Copy Markdown
Member

kermanx commented Apr 16, 2026

Hmm, it seems that this change would break the navigation. For example,

slidev/packages/client/composables/useNav.ts

Lines 211 to 216 in 1e056a4

function exitPresenter() {
router?.push({
path: getSlidePath(currentSlideNo.value, false),
query: { ...router.currentRoute.value.query },
})
}

relies on getSlidePath to return /presenter/1 instead of /BASE/presenter/1

@NgoQuocViet2001 NgoQuocViet2001 mentioned this pull request Apr 28, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@antfu antfu antfu left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@coaxial @kermanx @antfu